Conversation
202154b to
9d545d1
Compare
- audited file_cache.h
turbonfs/src/rpc_task.cpp
Outdated
| */ | ||
| inode->mark_commit_in_progress(); | ||
|
|
||
| assert(inode->is_commit_in_progress()); |
There was a problem hiding this comment.
this assert can theoretically fail if commit completes before we come here
turbonfs/src/rpc_task.cpp
Outdated
| * As we already 1GB worth of dirty data in the cache, we don't want | ||
| * to add more data to the cache. So we wait for the ongoing flush/commit. | ||
| */ | ||
| if (inode->is_commit_in_progress()) { |
There was a problem hiding this comment.
commit_state is protected by commit_lock_5 while we hold flush lock above. How is it safe?
i.e., if is_commit_in_progress() returns false and right after that commit is started by some other thread, can we wrongly proceed? and what happens if we proceed.
Remember we said flush_lock is for protecting any changes to the backend file size (either through flush or commit). Are we still following that?
turbonfs/src/rpc_task.cpp
Outdated
| /* | ||
| * Get the bcs marked for commit_pending. | ||
| */ | ||
| if (rpc_api->pvt == nullptr) { |
There was a problem hiding this comment.
do we ever expect caller to have already set the bc list?
turbonfs/inc/rpc_task.h
Outdated
| if (client->mnt_options.nfs_port == 2048) { | ||
| csched = stable_write ? CONN_SCHED_FH_HASH : CONN_SCHED_RR; | ||
| } |
There was a problem hiding this comment.
We don't need this new function, it's confusing.
What we need is simply this in the write task
if (!inode->is_stable_write()) {
set_csched(CONN_SCHED_RR);
}
basically, what we are saying is if it's an unstable write it can afford to use RR aka multiple connections, else whatever alloc_rpc_task() has chosen, use that.
786b534 to
fa05b1d
Compare
Audit nfs_inode.h Audit rpc_task.h
fa05b1d to
8610721
Compare
ed07eba to
f024ccb
Compare
No description provided.